repo: Only use mmap() for metadata > 16k
authorColin Walters <walters@verbum.org>
Thu, 1 Sep 2016 20:09:30 +0000 (16:09 -0400)
committerAtomic Bot <atomic-devel@projectatomic.io>
Thu, 8 Sep 2016 14:56:30 +0000 (14:56 +0000)
See http://stackoverflow.com/questions/258091/when-should-i-use-mmap-for-file-access
and
https://lwn.net/Articles/591978/

I didn't really notice much performance difference in some small
tests, but I happened to be stracing and realized we were `mmap()`ing
even for 50 bytes which is not very useful, so let's not do it.

Closes: #489
Approved by: alexlarsson

src/libostree/ostree-repo.c

index d33a96abf743f259fb3c8e4d0388278f8e5d066e..59bfbf9e71a580aa7ced7a28f43e1c1881009f17 100644 (file)
@@ -2544,6 +2544,7 @@ load_metadata_internal (OstreeRepo       *self,
 {
   gboolean ret = FALSE;
   char loose_path_buf[_OSTREE_LOOSE_PATH_MAX];
+  struct stat stbuf;
   glnx_fd_close int fd = -1;
   g_autoptr(GInputStream) ret_stream = NULL;
   g_autoptr(GVariant) ret_variant = NULL;
@@ -2565,23 +2566,39 @@ load_metadata_internal (OstreeRepo       *self,
 
   if (fd != -1)
     {
+      if (fstat (fd, &stbuf) < 0)
+        {
+          glnx_set_error_from_errno (error);
+          goto out;
+        }
+
       if (out_variant)
         {
-          GMappedFile *mfile;
+          /* http://stackoverflow.com/questions/258091/when-should-i-use-mmap-for-file-access */
+          if (stbuf.st_size > 16*1024)
+            {
+              GMappedFile *mfile;
 
-          mfile = g_mapped_file_new_from_fd (fd, FALSE, error);
-          if (!mfile)
-            goto out;
-          ret_variant = g_variant_new_from_data (ostree_metadata_variant_type (objtype),
-                                                 g_mapped_file_get_contents (mfile),
-                                                 g_mapped_file_get_length (mfile),
-                                                 TRUE,
-                                                 (GDestroyNotify) g_mapped_file_unref,
-                                                 mfile);
-          g_variant_ref_sink (ret_variant);
-
-          if (out_size)
-            *out_size = g_variant_get_size (ret_variant);
+              mfile = g_mapped_file_new_from_fd (fd, FALSE, error);
+              if (!mfile)
+                goto out;
+              ret_variant = g_variant_new_from_data (ostree_metadata_variant_type (objtype),
+                                                     g_mapped_file_get_contents (mfile),
+                                                     g_mapped_file_get_length (mfile),
+                                                     TRUE,
+                                                     (GDestroyNotify) g_mapped_file_unref,
+                                                     mfile);
+              g_variant_ref_sink (ret_variant);
+            }
+          else
+            {
+              GBytes *data = glnx_fd_readall_bytes (fd, cancellable, error);
+              if (!data)
+                goto out;
+              ret_variant = g_variant_new_from_bytes (ostree_metadata_variant_type (objtype),
+                                                      data, TRUE);
+              g_variant_ref_sink (ret_variant);
+            }
         }
       else if (out_stream)
         {
@@ -2589,15 +2606,10 @@ load_metadata_internal (OstreeRepo       *self,
           if (!ret_stream)
             goto out;
           fd = -1; /* Transfer ownership */
-          if (out_size)
-            {
-              struct stat stbuf;
-
-              if (!glnx_stream_fstat ((GFileDescriptorBased*)ret_stream, &stbuf, error))
-                goto out;
-              *out_size = stbuf.st_size;
-            }
         }
+
+      if (out_size)
+        *out_size = stbuf.st_size;
     }
   else if (self->parent_repo)
     {